Cleanup main model: Clean all includes and make some components to be forward declared by nitbharambe · Pull Request #1142 · PowerGridModel/power-grid-model

nitbharambe · 2025-09-29T15:23:37Z

Idea is to make components forward declared to reduce coupling.

This draft PR is created for gathering thoughts and requirements as of now.

Changes proposed in this PR include:

Forward declared all components
Removed unecessary exception and logging includes.

We shall maintain include discipline from now. Include order would be:

For include / source files

Important files (Main subjects). (Intention is to highlight for readability and not be strict)
Other required files
External dependencies
Standard library

For tests:

Helper includes of test
Important files to be tested (Main subjects). (Intention is to highlight for readability and not be strict)
Other required files
External dependencies
Testing dependencies
Standard library

In general, use <> for external. "" for internal. Note: PGM includes in test are treated as external, hence they go as <>.

Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>

figueroa1395 · 2025-10-01T14:30:05Z

I like the cleanup. Now I wonder, should we move all_components.hpp to the component folder? Also seems intuitive to me to rename component.hpp to all_components_fwd.hpp, or somehow homogenize the two.

In addition, can we also include component_list in this "revolution"? Perhaps it now belongs in component.hpp and we try to keep all component related stuff in one location only.

Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>

sonarqubecloud · 2025-10-20T09:34:54Z

Quality Gate passed

Issues
11 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>

figueroa1395

One additional thing is that perhaps #1142 (comment) is still relevant. Also, maybe triggering Copilot for review might be useful since missing things for a human due to the nature of this PR is very easy.

Rest looks good, just minor questions.

.clang-tidy

power_grid_model_c/power_grid_model/include/power_grid_model/common/three_phase_tensor.hpp

Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>

tests/cpp_unit_tests/test_node.cpp

tests/cpp_unit_tests/test_power_sensor.cpp

Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>

figueroa1395

I'm a fan of this! I just left a couple remarks.

figueroa1395 · 2026-02-03T15:26:43Z

power_grid_model_c/power_grid_model/include/power_grid_model/auxiliary/dataset.hpp

 #include "meta_data.hpp"

 #include "../common/common.hpp"
-#include "../common/counting_iterator.hpp"


As pointed out by the CI, add this via Jinja instead of directly editing here.

figueroa1395 · 2026-02-03T15:26:55Z

power_grid_model_c/power_grid_model/include/power_grid_model/auxiliary/output.hpp


 #include "../common/common.hpp"
-#include "../common/enum.hpp"


tests/native_api_tests/test_api_serialization.cpp

figueroa1395 · 2026-02-03T15:38:39Z

tests/native_api_tests/test_api_serialization.cpp


 #include <doctest/doctest.h>

+// NOLINTBEGIN(misc-include-cleaner)


I see many of these, probably related to nlohmann/json. Shouldn't we instead exclude it via misc-include-cleaner.IgnoreHeaders as we did with Eigen?

figueroa1395 · 2026-02-03T15:40:24Z

tests/native_api_tests/test_api_model_update.cpp

    ]
  }
-})json"s;
+})json"s; // NOLINT(misc-include-cleaner) https://github.com/llvm/llvm-project/issues/98122


These are annoying but I guess there is nothing else to do about them...

figueroa1395 · 2026-02-03T15:42:05Z

tests/cpp_validation_tests/test_validation.cpp

                auto const& attribute_name = MetaData::attribute_name(attribute_meta);
                // TODO need a way for common angle: u angle skipped for now
-                if (attribute_name == "u_angle"s) {
+                if (attribute_name == "u_angle"s) { // NOLINT(misc-include-cleaner)


Why this one?

Missed adding github issue name there. Same as others. Its with operator s

figueroa1395 · 2026-02-03T15:44:45Z

tests/cpp_unit_tests/test_meta_data_generation.cpp

        auto const& node = meta_map.get_component("node");
        auto const& node_attr = node.attributes;
-        CHECK(node_attr[0].name == "id"s);
+        CHECK(node_attr[0].name == "id"s); // NOLINT(misc-include-cleaner)


Why are these a problem here?

figueroa1395 · 2026-02-03T15:46:36Z

tests/cpp_unit_tests/test_math_solver_sc.cpp

+#include <power_grid_model/common/exception.hpp>
+#include <power_grid_model/common/grouped_index_vector.hpp>
+#include <power_grid_model/common/three_phase_tensor.hpp>
+#include <power_grid_model/math_solver/short_circuit_solver.hpp> // NOLINT(misc-include-cleaner)


Is it asking to directly import things called in the short circuit module? Or what's the issue?

tests/cpp_unit_tests/test_math_solver_pf_iterative_current.cpp

power_grid_model_c_example/main.c

Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>

mgovers

awesome progress

mgovers · 2026-02-06T07:56:35Z

.clangd

I find clangd giving a lot more insights than microsoft c++. It spotted many of the cleaned up things of header files which is not possible via clang-tidy.

I will bring up configuration in a knowledge sharing session.

in that case, maybe add the extension to the workspace recommendations?

Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>

sonarqubecloud · 2026-02-06T10:31:21Z

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

change to forward declarations

34ba5b2

Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>

nitbharambe self-assigned this Sep 29, 2025

nitbharambe added the improvement Improvement on internal implementation label Sep 29, 2025

nitbharambe added 5 commits October 19, 2025 11:13

change incldues

f889ae6

Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>

change incldues fix

9e8c714

Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>

add component test includes

5cf953d

Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>

add common test includes

be33aef

Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>

add eigen ignore

ad81da1

Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>

nitbharambe and others added 6 commits December 11, 2025 12:47

Merge branch 'main' into feature/component-fwd-cleanup

473994d

remove TODO

5418455

Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>

Merge branch 'main' into feature/component-fwd-cleanup

a463032

add new test file include

b054e9f

Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>

remove comments

424f032

Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>

fix assert missing

dffc4f4

Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>

nitbharambe marked this pull request as ready for review January 7, 2026 13:32

figueroa1395 reviewed Jan 8, 2026

View reviewed changes

.clang-tidy Outdated Show resolved Hide resolved

power_grid_model_c/power_grid_model/include/power_grid_model/common/three_phase_tensor.hpp Show resolved Hide resolved

nitbharambe and others added 2 commits January 12, 2026 09:26

rename to checkoptions

c3c6f95

Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>

Merge branch 'main' into feature/component-fwd-cleanup

924e928

mgovers reviewed Jan 12, 2026

View reviewed changes

tests/cpp_unit_tests/test_node.cpp Outdated Show resolved Hide resolved

tests/cpp_unit_tests/test_power_sensor.cpp Outdated Show resolved Hide resolved

clean pt 2

4a5b00c

Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>

nitbharambe marked this pull request as draft January 26, 2026 14:32

nitbharambe added 7 commits February 1, 2026 22:36

fix include search place

529d379

Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>

add more includes

b32792a

Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>

add more includes 2

2b6f670

Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>

add more includes 3

c59acaf

Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>

operator s includes

eee37dd

Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>

remove includes

21162c0

Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>

add definition option

986e1b2

Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>

nitbharambe and others added 7 commits February 2, 2026 17:14

round 3

6ee5cee

Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>

round 4

764373c

Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>

round 5

0160871

Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>

round 6

c8ac05d

Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>

Merge branch 'main' into feature/component-fwd-cleanup

dbca8bc

final includes

5ea9b17

Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>

final includes 2

eec172b

Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>

nitbharambe changed the title ~~Cleanup main model: Make components to be forward declared~~ Cleanup main model: Clean all includes and make some components to be forward declared Feb 3, 2026

figueroa1395 reviewed Feb 3, 2026

View reviewed changes

nitbharambe added 12 commits February 5, 2026 09:04

clean aux hpps

4ba9e23

Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>

clean common

3a6bcc6

Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>

clean components

68dbe62

Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>

clean main core

5bd405e

Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>

clean math solver

da6da16

Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>

remaining maincore minor

25ff542

Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>

clean optimizer

404309a

Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>

clean remaining

82e6abd

Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>

order tests

c654df3

Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>

clean c api

53342b4

Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>

minor addition

829e229

Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>

add reuse

950e953

Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>

mgovers reviewed Feb 6, 2026

View reviewed changes

remaining

af44f45

Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>


		#include "../common/common.hpp"
		#include "../common/enum.hpp"


		#include <doctest/doctest.h>

		// NOLINTBEGIN(misc-include-cleaner)

Conversation

nitbharambe commented Sep 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes proposed in this PR include:

Uh oh!

figueroa1395 commented Oct 1, 2025

Uh oh!

sonarqubecloud bot commented Oct 20, 2025

Quality Gate passed

Uh oh!

figueroa1395 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

figueroa1395 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mgovers left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Feb 6, 2026

Quality Gate passed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

nitbharambe commented Sep 29, 2025 •

edited

Loading